Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dv] Create and deploy reset agent #25463

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

martin-velay
Copy link
Contributor

No description provided.

Signed-off-by: Martin Velay <[email protected]>
- Not working yet !

Signed-off-by: Martin Velay <[email protected]>
- generate a reset on the fly after a certain delay only for debug
purpose. It should be removed and should probably happen in the base
test when rand_reset_vseq has been invoked.

Signed-off-by: Martin Velay <[email protected]>
@martin-velay
Copy link
Contributor Author

Hi DV team (@rswarbrick, @hcallahan-lowrisc, @antmarzam, @KinzaQamar)

Here is the PR I was talking about earlier today. I was expecting to have something in better to share by the end of this day but unfortunately things didn't went well. It compiles, elaborate, start to run and fail for a reason I cannot explain yet. I'll work on it later next week. This PR will be split into multiple later as I think these changes can happen incrementally. I haven't done that so far as I was not sure of the implementation.

You can start to have a look and give me feedback on some points which are supposed to be done at my end:

  1. About the reset agent itself.
  2. About the way I have started to make the reset shared with the other components through TLM interface.

-> Other comments are welcome either

What I still need to address:

  1. Make sure the current implementation works.
  2. Splitting the dv_lib into 3: dv_base_agent_lib, dv_base_env_lib and reset_agent.
  3. Managing multiple reset agents, as discussed it's a must have for some env.
  4. Trigger on the fly resets from the IP specific base test when rand_reset_vseq is called (not from the base test from dv_lib for the moment as not all the IP will move to this way directly, or maybe do it via a config bit somewhere).
  5. Address all my comments tagged with "TODO MVy" (BTW if you have some comments on these I' be happy to have inputs on some).

-> Do you see anything else?

When all these points will be addressed and will work:

  • Make all DV env transitioning to reset agent adoption.
  • This should simplify some piece of code as highlighted with the "TODO MVy". And it will also uniformize some points.
  • Remove the old reset triggering way.

-> Do you see anything else?

Note: @rswarbrick, I don't find Marcelo for some reason. Can you help me ?

- This commit should squashed
- Some derived agents are managing their reset signal and do some
different operation when the reset is asserted and deasserted. See to
convert these tasks into functions to make sure all will happn at the
same time

Signed-off-by: Martin Velay <[email protected]>
@rswarbrick
Copy link
Contributor

@marcelocarvalhoLowRisc (I don't seem to be able to add Marcelo as a reviewer but I'm not entirely sure why. Maybe because he hasn't had any PRs merged yet?)

@vogelpi
Copy link
Contributor

vogelpi commented Dec 6, 2024

@marcelocarvalhoLowRisc (I don't seem to be able to add Marcelo as a reviewer but I'm not entirely sure why. Maybe because he hasn't had any PRs merged yet?)

@marcelocarvalhoLowRisc was a member of our GitHub org but not part of the OpenTitan GitHub team. I've now invited him.

On a different note: I've talked to @martin-velay and as outlined in his description above this is a draft PR not intended to be merged as is. There is still work to be done to get this in a mergeable state (including splitting it up into smalller, reviewable chunks) but Martin wanted to give people a chance to take a look asap and if possible to provide early feedback to check this makes sense to others as well. I think this is actually perfectly fine but of course, once the implementation becomes more mature and this needs real review and merging, the size has to be reduced. Martin is aware of this.

FYI @rswarbrick , @mundaym .

- still not complete, it doesn't work yet but this activity needs to
be paused for the moment.

Signed-off-by: Martin Velay <[email protected]>
@martin-velay
Copy link
Contributor Author

martin-velay commented Dec 6, 2024

Status update:

I have pushed all my changes. This PR is still not ready, the reset is not happening as it should yet. I need to pause this activity to be focused on V3 activities.

What should be done next: find a way of disabling the main process when a reset is happening and then restart. This should probably imply to have a fork with a reset and main thread as done in many components, but this be done in the main virtual sequence. But the this main sequence shouldn't be killed by using the stop_sequences from the sequencer, otherwise will run at all. Instead, a task should be spawned from the main vseq and this task should be killed by calling disable fork and then restarted. This should happen as many as defined by a control knob which says how many resets should happen. By default this number will be equal to 1 (for initial reset). Add also a way to trigger a reset sequence on the fly from the main vseq.

Note to myself: run test
util/dvsim/dvsim.py hw/ip/hmac/dv/hmac_sim_cfg.hjson -t xcelium -i hmac_smoke -gd --fixed-seed=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants